Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue#566 #630

Merged
merged 9 commits into from
Oct 9, 2024
Merged

Issue#566 #630

merged 9 commits into from
Oct 9, 2024

Conversation

ugochukwu-850
Copy link
Contributor

In this PR,
I have made changes in the speed.rs module . Explaining how the entire module works
I have also added a note on the set speed function - Specifying there would be an increase of pitch

I have also removed the note comment from @dvdsk on the try_seek method : As the reason for the commit is now fixed

ugochukwu-850 added 2 commits October 5, 2024 10:24
Documented how playback speed mutation works and the effects with examples
@dvdsk
Copy link
Collaborator

dvdsk commented Oct 5, 2024

super! two small things, you need to run cargo fmt to fix the formatting and I would suggest moving some of the documentation to source/mod.rs specifically add it to fn speed. Since that is what most ppl will see.

@ugochukwu-850
Copy link
Contributor Author

@dvdsk , Ohh I forgot to run cargo fmt and yes your right . I should probably move the docs to the mod file, so it's more accessible.

Expect the commit, any hour 24hrs from now . I was just so busy enjoying the weekend . 😅.

@ugochukwu-850
Copy link
Contributor Author

Alright , I think this is good @dvdsk , please review and merge.

src/source/speed.rs Outdated Show resolved Hide resolved
src/source/speed.rs Outdated Show resolved Hide resolved
src/source/speed.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@dvdsk dvdsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few tiny issues otherwise looks good!

@dvdsk
Copy link
Collaborator

dvdsk commented Oct 7, 2024

I found a great resource on documenting items, I have added it to the CONTRIBUTING.md guidlines: https://github.com/RustAudio/rodio/blob/master/CONTRIBUTING.md#documentation

@ugochukwu-850
Copy link
Contributor Author

@dvdsk nice vets , I would take an extra look on the contribution guidelines you cited and by tomorrow , I should have this in optimum status .

Btw I wanted to propose a new feature on the output stream structure , could I create a new issue for that .

@dvdsk
Copy link
Collaborator

dvdsk commented Oct 8, 2024

Btw I wanted to propose a new feature on the output stream structure , could I create a new issue for that .

yes please do

@ugochukwu-850
Copy link
Contributor Author

@dvdsk Thanks for the resources you added to the contribution guidelines ; they were very useful . I think we are good now , please review and merge if its ok.

@@ -412,6 +412,7 @@ where
}

/// Changes the play speed of the sound. Does not adjust the samples, only the play speed.
/// Creates a Speed struct that handles the speed control
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you replace this with the documentation from the Sink? I especially would like the note that the pitch will change. Finally could you note how the pitch will change (so at 0.5 speed is it higher or lower). That is something that might not be clear for readers of the doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dvdsk I have made the required changes , I also added some part of the doc to the sink.set_speed function to increase accessibility . Please review and merge

ugochukwu-850 added 2 commits October 9, 2024 17:59
I added docs on the effects of changing the speed on both total_duration and pitch on both the source.speed trait and sink set_speed functions .
This should help users understand what each function does
I added docs on the effects of changing the speed on both total_duration and pitch on both the source.speed trait and sink set_speed functions .
This should help users understand what each function does
@dvdsk dvdsk merged commit d1bd852 into RustAudio:master Oct 9, 2024
11 checks passed
@dvdsk
Copy link
Collaborator

dvdsk commented Oct 9, 2024

thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants